Skip to content

Fix bug with SignChecker when NaNs are present#256

Merged
ph-kev merged 1 commit intomainfrom
kp/bugs
Oct 29, 2025
Merged

Fix bug with SignChecker when NaNs are present#256
ph-kev merged 1 commit intomainfrom
kp/bugs

Conversation

@ph-kev
Copy link
Member

@ph-kev ph-kev commented Oct 23, 2025

The original code is nanmean(var.data .> 0). This does not work, because NaN > 0 is false and not NaN.

closes #254

@ph-kev ph-kev requested a review from nefrathenrici October 23, 2025 00:06
Copy link
Member

@nefrathenrici nefrathenrici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!

ext/checkers.jl Outdated
Comment on lines 267 to 274
positive_count = 0
count = 0
for val in var.data
isnan(val) && continue
count += 1
positive_count += val > 0
end
sim_pos_proportion = positive_count / count
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be simpler:

Suggested change
positive_count = 0
count = 0
for val in var.data
isnan(val) && continue
count += 1
positive_count += val > 0
end
sim_pos_proportion = positive_count / count
valid = @. !isnan(var.data)
sim_pos_proportion = sum((var.data .> 0) .& valid) / sum(valid)

The original code is `nanmean(var.data .> 0)`. This does not work,
because `NaN > 0` is `false` and not `NaN`.
@ph-kev ph-kev merged commit 23683d4 into main Oct 29, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug with SignChecker

2 participants